Skip to content

[이찬주] Week5#214

Open
ckswnskfk wants to merge 12 commits intocodeit-bootcamp-frontend:part1-이찬주from
ckswnskfk:part1-이찬주-week5

Hidden character warning

The head ref may contain hidden characters: "part1-\uc774\ucc2c\uc8fc-week5"
Open

[이찬주] Week5#214
ckswnskfk wants to merge 12 commits intocodeit-bootcamp-frontend:part1-이찬주from
ckswnskfk:part1-이찬주-week5

Conversation

@ckswnskfk
Copy link
Copy Markdown
Collaborator

요구사항

기본

sign in 페이지

sign up 페이지

  • 이메일 중복 확인은 “/api/check-email” POST 요청해서 확인합니다.
(중복된 이메일 확인은 “test@codeit.com”을 활용해 주세요.)
  • 유효한 회원가입 형식의 경우 “/api/sign-up” POST 요청하고 성공 응답을 받으면 “/folder”로 이동합니다.

심화

  • 로그인/회원가입시 성공 응답으로 받은 accessToken을 로컬 스토리지에 저장합니다.
  • 로그인/회원가입 페이지에 접근시 로컬 스토리지에 accessToken이 있는 경우 “/folder” 페이지로 이동합니다.

주요 변경사항

  • fetch를 다루는 공통 객체 fetchApi 생성
  • 각 페이지(도메인) 별 api.js 파일에서 fetchApi 호출하기로 함

멘토에게

  • 막판엔 급하게 한 감이 있어서 코드가 어떤지 잘 모르겠습니다.. 개선 사항 팍팍 말씀해주셔요!

@ckswnskfk ckswnskfk requested a review from sstaar91 November 19, 2023 17:42
@ckswnskfk ckswnskfk added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Nov 19, 2023
Copy link
Copy Markdown
Collaborator

@sstaar91 sstaar91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

찬주님! 5주차 과제 하시느라 수고 많으셨습니다!
이번 과제 제출 코드에서는 유독 깜짝 놀라는 코드들이 많았습니다!
이미 어딘가에서 근무를 하고 오신게 아닌가 생각이 들 정도로 구현한 코드가 있어서 놀랐네요..!
제가 찬주님과 같은 시절을 생각해보면 굉장히 부끄러워지기도 합니다...ㅎㅎ

전반적으로 보기에도 깔끔했고, 특히 fetchApi.js 파일을 만들어서 사용하신 부분은
차후 React에서 custom Hook이라는 개념과도 밀접한 관련이 있기 때문에
지금 적용해보신 이 경험이 매우 큰 도움이 될 거라고 생각합니다!

Comment thread index.js
Comment on lines +14 to +21
function navigateFolderPage(href) {
if (localStorage.getItem("accessToken")) {
location.href = "/pages/folder/";
return;
}

location.href = href;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 이런 방법도 생각해 볼 수 있겠군요! 좋습니다 ㅎㅎ

나중 얘기지만 도움이 될 수 있을 부분일거 같아 미리 말씀드리면
통신 과정에서 사용자의 인증을 위해 헤더에 토큰을 넣어서 함께 보내는 경우가 많이 있습니다.
이 과정에서 axios를 이용한 인터셉터를 적용할 수 있는데,
인터셉터는 통신 전 해당 요청을 가로채서 어떠한 작업을 거친 후 다시 통신을 보내는 방식을 뜻합니다.

지금 구현한 기능과는 조금 방식이 다르지만, 어쨋든 지금도 페이지 이동 전 토큰 유무를 확인한 뒤
그에 따라 페이지를 이동하는 것이라서 어떻게 보면 작은 인터셉터라고도 할 수 있겠네요 ㅎㅎㅎ
갑자기 생각나서 해당 내용에 대해 전달드려봅니다..ㅎㅎ

인터셉터와 관련된 글도 링크로 전달드릴테니, 차후에 한번 적용해보세요!
https://www.timegambit.com/blog/digging/axios/01

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 좀 더 확실한 방법이겠네요!
제가 한 방식은 직접 url을 입력해서 접근하는 방식에는 통하지 않아서..ㅋㅋㅋ
지식 전수 감사합니다!:smile:

Comment thread pages/auth/sign.js Outdated
Comment on lines +21 to +24
function getInputsErrMsgEl(target) {
const parent = target.parentElement;
return parent.querySelector(".input-error__message");
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 함수를 분리한 건 어떤 이유일까요?
실제 코드에서 사용하신건 handleErrorMessage 에서 밖에 없어서
해당 함수의 분리는 조금은 불필요하다고 생각이 되네요!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아차차 그런가요?
handleErrorMessage 함수 자체가 좀 복잡하다고 생각돼서 리펙터링을 하고는 싶은데 잘 안돼서 나온 결과인 것 같습니다.. 수정하겠습니다!

Comment thread pages/auth/signin/signin.js Outdated
Comment on lines +14 to +16
if (localStorage.getItem("accessToken")) {
location.replace("/pages/folder/");
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위에 index.js에서 이미 해당 로직에 대한 부분을 작성해 두셨기 때문에
해당 코드는 불필요한 부분이라고 보여집니다.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

직접 signin 페이지나 signup 페이지의 url로 접근하는 경우도 있을 수 있다고 생각해 추가한 코드인데요, 없는 게 더 좋을까요?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위에서 만든 navigateFolderPage 함수를 사용하는 방법도 있겠죠?
해당 함수를 import 한 다음 해당 파일에서 호출할 때, 경로만 입력해주면 동일한 동작을 할 것 같습니다.

Comment thread scripts/fetchApi.js
@@ -0,0 +1,43 @@
const BASE_URL = "https://bootcamp-api.codeit.kr/api";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 굿입니다 👍👍

Comment thread scripts/fetchApi.js

post: async function (url, { body, headers }) {
const options = {
method: "POST",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

POST 라는 method는 데이터를 전달하겠다라는 의미이기 때문에
body에 데이터가 담기지 않는 경우는 없다고 생각하셔도 좋습니다.
그래서 null이라는 옵션을 사용하는건 불필요하다고 생각이 되네요!

Comment thread scripts/fetchApi.js
method: "POST",
body: body ? JSON.stringify(body) : null,
headers: {
"Content-Type": "application/json",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

content-type도 json 형태의 데이터만 보내는 것이 아닐 수 있습니다.
대표적으로 form-data 형태가 있는데, 이미지를 같이 보내는 경우 사용하는 content-type이라서
만약 확장성 있게 POST를 사용한다고 하면 이 부분도 옵션으로 주는 것이 좋습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants